Skip to content

Name a declaration from a path-mapped assembly from its root once - #20518

Open
xperiandri wants to merge 6 commits into
dotnet:mainfrom
xperiandri:fix/pathmap-declaration-location
Open

xperiandri wants to merge 6 commits into
dotnet:mainfrom
xperiandri:fix/pathmap-declaration-location

Conversation

@xperiandri

@xperiandri xperiandri commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

Go To Definition into a sibling project of a solution built with a path map — the <PathMap>$(MSBuildThisFileDirectory)=.\</PathMap> plus DeterministicSourcePaths that many Directory.Build.props set — opens generated metadata instead of the source, because the declaration names the compile-time directory twice:

.\src\Domain\.\src\Domain\Types.fs

A mapped build writes both the file names of its ranges and its working directory into F# metadata relative to the same root, which the assembly never records. SymbolHelpers.fileNameOfItem joins the two on the assumption that the directory is absolute and the file name relative to it, so the directory is repeated. It now joins only when the directory is rooted; otherwise the file name is as complete as the metadata can make it and is returned unchanged. A build without a map keeps an absolute directory and takes the same path as before, and an empty SourceCodeDirectory, which shared and IL-only CCUs use, behaves as it did.

#20470 keeps the map out of the options the IDE builds, which covers references between projects held in memory. It cannot cover this: a referenced assembly on disk was built by MSBuild with the map and carries the mapped names permanently. The editor still has to resolve a relative name against the solution rather than the process's current directory; that is #20519.

Stacked on #20476, which adds FSharpChecker/PathMap.fs; the diff shrinks to its own commit once that merges.

🤖 Generated with Claude Code

@github-actions

github-actions Bot commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

❗ Release notes required

You can open this PR in browser to add release notes: open in github.dev


✅ Found changes and release notes in following paths:

Change path Release notes path Description
`src/Compiler` docs/release-notes/.FSharp.Compiler.Service/11.0.100.md

@github-actions github-actions Bot added the ⚠️ Affects-Design-Time Tooling check: PR touches type providers or dependency manager label Sep 11, 2026
@github-actions

This comment has been minimized.

xperiandri added a commit to xperiandri/fsharp that referenced this pull request Sep 11, 2026
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

@T-Gro T-Gro left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@github-project-automation github-project-automation Bot moved this from New to In Progress in F# Compiler and Tooling Sep 11, 2026
@T-Gro T-Gro added the AI-reviewed PR reviewed by AI review council label Sep 11, 2026
@T-Gro
T-Gro self-requested a review September 11, 2026 12:45
xperiandri and others added 6 commits September 11, 2026 17:51
…imports are cached

FrameworkImportsCache keys the framework imports, and the TcGlobals built
with them, by the framework set alone. A project reusing the entry got a
fresh TcGlobals only when langVersion or realsig differed, and even then took
pathMap from the cached instance. Since TypedTreePickle applies that map to
every range it writes, the in-memory reference data of each project carried
the --pathmap of whichever project filled the cache first, and a project
without a map handed its consumers file names nothing on disk matches.

pathMap now takes part in the decision like langVersion and realsig, and the
new TcGlobals takes it from the project's own TcConfig, in the incremental
builder and the transparent compiler alike.

Fixes dotnet#20474

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ing project

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
A build that maps its source paths - as DeterministicSourcePaths does - maps the
directory it compiled in along with the file names of its ranges, so the two
reach the same root and neither records what that root is. `fileNameOfItem`
joined them anyway, on the assumption that the directory is absolute and the
file name relative to it, and the declaration of a symbol imported from such an
assembly came back naming the directory twice. Nothing matches a path like that,
so Go To Definition reported the symbol as external and opened generated
metadata instead of its source.

Join only when the directory is rooted. When it is not, the file name already
reaches the root on its own and is as complete as the metadata can make it.

The IDE drops the path map from the options it builds, so this is reached
through an assembly on disk - which is where a path map is normally applied, and
which no change to those options can rewrite.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
On Linux and macOS a backslash is part of a file name, so a map to .\ left the
declaration's name unresolvable against the root there.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@xperiandri
xperiandri force-pushed the fix/pathmap-declaration-location branch from 7cf1ff0 to a4ae807 Compare September 11, 2026 16:16

@T-Gro T-Gro left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖🕵️ Please make the description more concise using this guidance. Keep the problem and why the change is needed prominent, using short sentences in simplified technical English. Retain necessary caveats.

@github-actions

Copy link
Copy Markdown
Contributor

🔍 Tooling Safety Check — Affects-Design-Time
Affects-Design-Time: Changes declaration navigation paths used by IDE tooling.

Generated by PR Tooling Safety Check · gpt56 1.7M ·

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

⚠️ Affects-Design-Time Tooling check: PR touches type providers or dependency manager AI-reviewed PR reviewed by AI review council

Projects

Status: In Progress

Development

Successfully merging this pull request may close these issues.

2 participants